Skip to content

[UR][Offload] Implement (most) kernel info queries #19806

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: sycl
Choose a base branch
from

Conversation

RossBrunton
Copy link
Contributor

Almost all of the required kernel info queries have been implemented.
The only exception is NUM_ARGS, which isn't reported by liboffload,
CUDA or HSA (the UR plugins return the current number of arguments,
not the total). This test has been marked as a known failure and tests
that use it as a placeholder value have been changed to use NAME.

Almost all of the required kernel info queries have been implemented.
The only exception is `NUM_ARGS`, which isn't reported by liboffload,
CUDA or HSA (the UR plugins return the *current* number of arguments,
not the total). This test has been marked as a known failure and tests
that use it as a placeholder value have been changed to use NAME.
case UR_KERNEL_INFO_ATTRIBUTES:
return ReturnValue("");
case UR_KERNEL_INFO_NUM_ARGS:
// This is unimplementable on liboffload (and AMD/Nvidia in general)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is implementable in the adapter, hKernel->Args->Storage.size() (or something to that effect)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I just read the PR description. I think if we're being more strict with the meaning then CUDA and HIP should be marked unsupported as well.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And actually if what the CUDA and HIP adapters do is good enough for SYCL just now then maybe we can relax the UR spec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants